From 0f563cc08e92e3d762ff8d072a4fa5c782ece8e7 Mon Sep 17 00:00:00 2001 From: tsteven4 <13596209+tsteven4@users.noreply.github.com> Date: Fri, 28 Aug 2020 05:50:07 -0600 Subject: [PATCH] Fix potential segmentation faults with exif. (#639) 1. A pointer exif_app_ to an ExifApp on QList exif_apps was saved. After this the list was modified. This can lead to the saved pointer becoming invalid. 2. The ExifApp structure had a dtor, but no other special functions, i.e. it violated the rule of 3 and the rule of 5. Operations on QList may cause an ExifApp on the list to be copied or destroyed. If an ExifApp is destroyed, then the ExifApp dtor would close the files, even though there could be a copy of the ExifApp expecting the files still to be open. This scenerio occured with Qt6, causing segmentation faults in exif.test. --- exif.cc | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/exif.cc b/exif.cc index d2f71a7f5..783406fd8 100644 --- a/exif.cc +++ b/exif.cc @@ -177,20 +177,10 @@ struct ExifApp { gbfile* fcache{nullptr}; gbfile* fexif{nullptr}; QList ifds; - - ~ExifApp() - { - if (fcache) { - gbfclose(fcache); - } - if (fexif) { - gbfclose(fexif); - } - } }; static gbfile* fin_, *fout_; -static QList* exif_apps; +static QList* exif_apps; static ExifApp* exif_app_; static const Waypoint* exif_wpt_ref; static QDateTime exif_time_ref; @@ -345,6 +335,15 @@ exif_read_datestamp(const ExifTag* tag) static void exif_release_apps() { + for (auto* app : qAsConst(*exif_apps)) { + if (app->fcache) { + gbfclose(app->fcache); + } + if (app->fexif) { + gbfclose(app->fexif); + } + delete app; + } delete exif_apps; exif_apps = nullptr; } @@ -375,8 +374,8 @@ exif_load_apps() exif_app_ = nullptr; while (! gbfeof(fin_)) { - exif_apps->append(ExifApp()); - ExifApp* app = &exif_apps->last(); + exif_apps->append(new ExifApp); + ExifApp* app = exif_apps->last(); app->fcache = gbfopen(nullptr, "wb", MYNAME); app->marker = gbfgetuint16(fin_); @@ -1360,8 +1359,7 @@ exif_write_apps() { gbfputuint16(0xFFD8, fout_); - for (auto& app_instance : *exif_apps) { - ExifApp* app = &app_instance; + for (auto* app : qAsConst(*exif_apps)) { gbfputuint16(app->marker, fout_); @@ -1476,7 +1474,7 @@ static void exif_rd_init(const QString& fname) { fin_ = gbfopen_be(fname, "rb", MYNAME); - exif_apps = new QList; + exif_apps = new QList; } static void @@ -1508,7 +1506,7 @@ exif_wr_init(const QString& fname) exif_success = 0; exif_fout_name = fname; - exif_apps = new QList; + exif_apps = new QList; fin_ = gbfopen_be(fname, "rb", MYNAME); is_fatal(fin_->is_pipe, MYNAME ": Sorry, this format cannot be used with pipes!"); -- 2.30.2